Skip to content

Ensure copy* intrinsics also perform the static self-init checks #142575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 16, 2025

fixes #142532

r? @RalfJung

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 2025

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

@oli-obk oli-obk added the relnotes Marks issues that should be documented in the release notes of the next release. label Jun 16, 2025
@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 16, 2025

Adding relnotes as this will break stable code if someone wrote such code in the last year, but there is absolutely no reason to do so, so not even gonna crater it.

@workingjubilee
Copy link
Member

It's a soundness fix and is only reachable by having incorrect unsafe code that should be rejected by const eval anyway, right? Then we're justified in "break first, figure out how to migrate smoothly later".

@theemathas
Copy link
Contributor

In theory, this could also affect code where there's correct unsafe code in one crate, and there's incorrect safe code in another crate misusing the first crate.

@oli-obk oli-obk force-pushed the sneaky-self-init branch from 16daf7d to 04f835f Compare June 17, 2025 15:07
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with comment tweaks

Comment on lines +1416 to 1422
// Do our own "do not read from static items while initializing them" checks
if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(src, size.bytes() as i64) {
M::before_alloc_read(self, alloc_id)?;
}

// For the overlapping case, it is crucial that we trigger the read hook
// before the write hook -- the aliasing model cares about the order.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Do our own "do not read from static items while initializing them" checks
if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(src, size.bytes() as i64) {
M::before_alloc_read(self, alloc_id)?;
}
// For the overlapping case, it is crucial that we trigger the read hook
// before the write hook -- the aliasing model cares about the order.
// Trigger read hooks.
// For the overlapping case, it is crucial that we trigger the read hooks
// before the write hook -- the aliasing model cares about the order.
if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(src, size.bytes() as i64) {
M::before_alloc_read(self, alloc_id)?;
}

The core interpreter doesn't know what the hook does, so it seems odd to refer to to the static item recursion check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std::ptr::copy() can read from uninitialized statics (unsound)
5 participants